Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ci: added publishing intel trust authority AS docker #410

Merged
merged 2 commits into from
Jun 24, 2024

Conversation

pawelpros
Copy link
Contributor

@pawelpros pawelpros commented Jun 12, 2024

  • Refactored directory structure for building KBS docker images
  • Added publishing KBS intel trust authority AS docker image on ghcr.io

@fitzthum
Copy link
Member

fitzthum commented Jun 12, 2024

DCO check complains because the author does not match the sign-off. Basically it should match git config --local user.name. You can update that config and then do git commit --amend --reset-author I think.

@fitzthum
Copy link
Member

You should probably update the release script to make sure your staged image gets copied over as a release image.

@pawelpros pawelpros force-pushed the gh-ita-build-publish branch 2 times, most recently from 3c9266d to 67d375b Compare June 14, 2024 08:12
@pawelpros
Copy link
Contributor Author

Thanks for the tip - should be ok right now - I have also aligned with recent changes

Copy link
Member

@fitzthum fitzthum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine. I don't see anything bumping OpenSSL

Also note that we use the same naming convention for Dockerfiles in the AS dir. Do we care about this being consistent?

hack/release-helper.sh Show resolved Hide resolved
hack/release-helper.sh Outdated Show resolved Hide resolved
Copy link
Member

@fidencio fidencio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For change 1.

Why are we re-structuring the directories? What's the clear benefit of this new method over the older one?

I'd mostly be interested on whether you hit some issue with the current status of things, or not.

This kind of changes, unless those are fully triggered from the Makefile, deserve a release notes entry, as it changes files location that packagers may rely on.

.github/workflows/as-build-and-push.yaml Outdated Show resolved Hide resolved
.github/workflows/as-dockerbuild.yml Outdated Show resolved Hide resolved
.github/workflows/kbs-build-and-push.yaml Outdated Show resolved Hide resolved
attestation-service/docs/grpc-as.md Outdated Show resolved Hide resolved
attestation-service/docs/restful-as.md Outdated Show resolved Hide resolved
attestation-service/rvps/README.md Outdated Show resolved Hide resolved
docker-compose.yml Outdated Show resolved Hide resolved
docker-compose.yml Outdated Show resolved Hide resolved
docker-compose.yml Outdated Show resolved Hide resolved
Copy link
Member

@fidencio fidencio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change 2 is good, but I'd recommend doing it as part of its own commit(s).

.github/workflows/as-build-and-push.yaml Show resolved Hide resolved
.github/workflows/kbs-build-and-push.yaml Show resolved Hide resolved
.github/workflows/kbs-build-and-push.yaml Outdated Show resolved Hide resolved
Copy link
Member

@fidencio fidencio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now, about the change 3, which also deserves to be part of its own commit (still on this very same PR).

We're adding the possibility to build the ITA image, but ... correct me if I'm mistaken ... I think we're doing multi-arch build for all the images. With this in mind, an obvious question is ... is ITA supposed to support multi-arch? Or is it x86_64 specific?

Depending on the answer, we may have to limit this to only be built with for x86_64.

.github/workflows/kbs-build-and-push.yaml Show resolved Hide resolved
.github/workflows/kbs-build-and-push.yaml Outdated Show resolved Hide resolved
.github/workflows/kbs-docker-build.yml Outdated Show resolved Hide resolved
hack/release-helper.sh Show resolved Hide resolved
hack/release-helper.sh Show resolved Hide resolved
kbs/docker/intel-trust-authority/Dockerfile Outdated Show resolved Hide resolved
release-guide.md Show resolved Hide resolved
release-guide.md Outdated Show resolved Hide resolved
Signed-off-by: Pawel Proskurnicki <[email protected]>
Copy link
Member

@fidencio fidencio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, thanks @pawelpros!

Copy link
Member

@fitzthum fitzthum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Xynnn007 Xynnn007 merged commit 7e0d4d0 into confidential-containers:main Jun 24, 2024
15 checks passed
@pawelpros pawelpros deleted the gh-ita-build-publish branch July 8, 2024 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants